Add source map generation to bin/jsx #833
Conversation
ghost
assigned 
|
I bet @benjamn can add something to commoner to make this easier so you don't have to hack it. |
|
Have an issue filed to support this better: https://github.com/benjamn/commoner/issues/49 |
| ); | ||
| var code = transformed.code; | ||
| if (this.options.sourceMap) { | ||
| var mapFilename = writeSourceMap( |
There was a problem hiding this comment.
As noted on twitter, this is unsafe because the .process callback is only called the first time a module is built, so source map files will be written correctly for clean builds but not for incremental rebuilds.
Commoner needs to know about additional files before any processing takes place, so that it can make sure they get into the output directory, whether or not processing was necessary.
|
To be clear, I'd like to hold off on merging this pull request until Commoner has better support for writing multiple files per source module. I would be willing to accept a version of this pull request that only appended the encoded source map to the end of the module (without writing a separate file), however. |
|
I think starting with embedded sourcemaps is a good idea. Any idea how something like browserify handles them? (interested for our own purposes...) |
|
Well, browserify can presumably write one source map for the entire bundle, which is somewhat easier than writing individual source maps for each module. It also has an option for embedding the map in the bundle file. |
|
I was wondering what if you knew what it would do if each of the files it was putting into the bundle had an embedded map. No worries if you don't know! |
|
Ah, I see. That definitely is relevant to our interests… |
|
It seems that Browserify has good source map support: http://thlorenz.com/blog/browserify-sourcemaps It will extract source maps from source files and combine into a single source map for the bundle. I'm happy to switch to only generating embedded source maps at the moment. Then once Commoner has been enhanced, we could add a way to toggle between embedded and external file. e.g. |
|
The above commit generates inline source maps instead of files. So it should work well with Browserify. |
See [Commoner's README.md]( https://github.com/benjamn/commoner#generating-multiple-files-from-one-source-module) for further explanation of the new functionality. Among other potential benefits, this upgrade allows us to generate source maps as standalone files rather than appending them inline to every compiled module file under `build/modules/` (see facebook#833).
New `bin/jsx` options: ``` --source-map-inline ``` Generates inline source maps, useful for working with browserify. ``` --source-map-files ``` Generates a `.js.map` file for each generated `.js` file.
|
I've updated this now that Commoner supports multiple file generation (thanks to @benjamn ). New command line options for |
|
Ping. Also, do we want to support |
|
We probably want the same logic for |
|
I think browserify only works with inline source maps. |
|
What's happening here? I would love source maps, especially since we're doing it in the browser transform. |
|
|
|
I'm not clear on what |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
I actually would say that you can ignore |
|
I bet you could reuse some of the code from #910 now that it's merged. We also have some similar inline sourcemap code in https://github.com/facebook/react/blob/master/vendor/browser-transforms.js as well. We could probably start to share a bit (not super high-pri for me, but good form anyway) |
|
Hey @andrewdavey, I appreciate you taking a stab at this, but I'm going to close it out and bring in #1825 which gets us part of the way there. I would still love to see external soucemaps so if you get a chance to come back to this, let me know! PS, the work you did here definitely made it easier for me to write mine, so I appreciate it :) |
Work in progress for #766
Assuming a directory contains
foo.jsx, runningjsx -x jsx --source-map . .will generatefoo.jsandfoo.js.map.The generated
foo.jshas the//# sourceMappingURL=foo.js.mapcomment appended.Current limitations: